Skip to content

fix(desktop): let Windows users enable notifications after a false denial - #2483

Open
BradGroux wants to merge 1 commit into
block:mainfrom
BradGroux:agent/windows-notification-permission-recovery
Open

fix(desktop): let Windows users enable notifications after a false denial#2483
BradGroux wants to merge 1 commit into
block:mainfrom
BradGroux:agent/windows-notification-permission-recovery

Conversation

@BradGroux

@BradGroux BradGroux commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Addresses #2445.

What Windows users saw

Buzz could report desktop notification permission as denied before Windows had registered the installed app as a notification sender. The settings toggle then treated that value as final, so users could not turn notifications on even though requesting permission would repair the state.

What changed

  • default still requests access on every platform.
  • A false denied state gets one recovery request only in the Windows Tauri app.
  • Genuine denial remains terminal on macOS, Linux, and ordinary Windows web pages.
  • Unit coverage exercises the platform/runtime policy.
  • The settings-flow Playwright regression explicitly identifies itself as Windows Tauri and verifies one recovery request.

There is no retry loop, Tauri plugin fork, new dependency, or change to notification delivery.

Verification

  • just desktop-check desktop-test desktop-typecheck desktop-build: passed, including 3,889 JavaScript tests.
  • pnpm --dir desktop build:e2e: passed.
  • pnpm exec playwright test profile.spec.ts --project=integration --grep "Windows retries a false denied notification permission from settings": 1 passed.

What still needs a real Windows test

The automated regression uses a Windows platform identity and the real settings hook against the notification shim. It does not run inside Windows WebView2 or a packaged Tauri application.

Keep #2445 open until an affected Windows 11 installation confirms the request returns granted, Buzz appears under System > Notifications, and a real toast arrives.

@BradGroux
BradGroux requested a review from a team as a code owner July 23, 2026 07:06
@BradGroux BradGroux changed the title fix(desktop): recover Windows notification permission Let Windows users retry notifications after a false permission denial Jul 23, 2026
@BradGroux BradGroux changed the title Let Windows users retry notifications after a false permission denial Let Windows users turn on notifications after a false denial Jul 23, 2026
@BradGroux
BradGroux force-pushed the agent/windows-notification-permission-recovery branch from 500753b to 071c0b6 Compare July 28, 2026 16:40
@BradGroux

Copy link
Copy Markdown
Contributor Author

I rebased this branch onto current main and tightened the workaround after rechecking the actual trust boundary.

The reported Windows Tauri behavior is still relevant, but the old condition retried denied permission on any Windows browser. That would turn a genuine web denial into another prompt attempt and exceeded the issue's installed-app scope.

Head acda0d989a3d91eef16b3278a9afea43db3d732c now permits the one-time recovery request only when both Windows and Tauri are true. The Playwright regression also declares a Windows Tauri identity explicitly; this caught the overly broad condition during verification.

All 3,725 Desktop JavaScript tests, Desktop typecheck, the E2E build, the focused Playwright settings flow, and targeted Biome checks passed. A real Windows 11/WebView2 smoke test is still required, so #2445 should remain open and the PR should not claim packaged-shell proof.

@BradGroux BradGroux changed the title Let Windows users turn on notifications after a false denial fix(desktop): let Windows users enable notifications after a false denial Jul 29, 2026
@BradGroux
BradGroux force-pushed the agent/windows-notification-permission-recovery branch from 071c0b6 to 9a0f18d Compare July 30, 2026 14:37
@BradGroux

Copy link
Copy Markdown
Contributor Author

Rebased this branch onto current block/buzz main at c55e421a0.

Head moved from 071c0b63a to 9a0f18df9. git range-diff shows the Windows notification recovery patch is unchanged. The branch also cleanly incorporates the current profile E2E changes from main; no branch behavior was dropped during the rebase.

Verification:

  • Desktop Biome, file-size, text-size, and pubkey-truncation checks passed.
  • The full Desktop test suite passed.
  • TypeScript typecheck and the production Vite build passed on the final rebased branch.
  • git diff --check, DCO trailer checks, and the direct merge-base audit passed.

GitHub checks are rerunning on the new head.

@BradGroux
BradGroux force-pushed the agent/windows-notification-permission-recovery branch from 9a0f18d to f87a893 Compare August 1, 2026 01:12
@BradGroux

Copy link
Copy Markdown
Contributor Author

Rebased onto current block/buzz main at b1b283cd4. #2483 remains the only Buzz pull request addressing the Windows Tauri false-denial path, and no review feedback or superseding implementation surfaced.

Published head: f87a89336e3eb74db56710c1f7ec1029178bdf96.

Desktop check, typecheck, build, and all 3,889 JavaScript tests pass. The focused settings-flow Playwright regression also passes in Chromium.

The remaining boundary is unchanged: the automated regression exercises a Windows Tauri identity and the real settings hook against the notification shim, but it is not packaged Windows 11 or WebView2 proof. #2445 should remain open until an affected installation confirms registration and toast delivery.

@BradGroux
BradGroux force-pushed the agent/windows-notification-permission-recovery branch from f87a893 to aebe3f2 Compare August 4, 2026 20:40
@BradGroux

Copy link
Copy Markdown
Contributor Author

Rebased onto block/buzz main at ce3cf3cd2591f132f286fbc0a42a9e6699d0b08d and rechecked the Windows permission path, current notification settings code, open issue #2445, and competing pull requests.

The PR remains valid. Current main still requests notification access only from the default state and treats a Windows Tauri denied result as terminal. No other open or merged pull request implements the one-time installed-app recovery path. The branch remains narrowly guarded by both Windows and Tauri, so ordinary Windows web pages and genuine denial on other platforms do not receive another prompt attempt.

The rebase was clean and preserves the current profile E2E bridge changes. No production-path correction beyond the existing single recovery request was needed.

Exact-head verification on aebe3f29db29cbf5fad962447abb481773e568e0:

  • Desktop policy/static checks passed, including formatting and file-size, text-size, and pubkey-truncation gates.
  • The full Desktop JavaScript suite passed with 0 failures.
  • Desktop typecheck and production build passed.
  • The E2E build passed.
  • Focused Playwright coverage passed for “Windows retries a false denied notification permission from settings,” including the assertion that exactly one recovery request occurs.
  • git diff --check, conflict-marker scan, merge-base audit, and required DCO trailers passed.

The acceptance boundary is unchanged and should stay explicit: this automated regression uses a Windows Tauri identity and the real settings hook against the notification shim, but it is not packaged Windows 11 or WebView2 proof. #2445 should remain open until an affected installation confirms app registration, granted permission, and real toast delivery.

@BradGroux
BradGroux force-pushed the agent/windows-notification-permission-recovery branch from aebe3f2 to d33c71c Compare August 5, 2026 22:28
@BradGroux

Copy link
Copy Markdown
Contributor Author

Review and rebase summary

Reviewed the PR for accuracy against current block/buzz main at d42d60d64 and rebased the branch onto that commit.

What this PR does

Adds a Windows-specific retry path for desktop notification permissions. On Windows Tauri, the WebView2 notification shim can report denied even when the OS permission is actually default (a known false-denial). The new ensureDesktopNotificationPermission function retries requestAccess() when the current permission is denied and the environment is Windows Tauri. On non-Windows or non-Tauri environments, denied is terminal and no retry is attempted. The default state still requests access on every platform.

Accuracy review

  • The isWindowsPlatform() helper uses navigator.platform matching /^win/i, which correctly identifies Win32 and Win64 without false-positive on Darwin or Linux x86_64.
  • The ensureDesktopNotificationPermission function is cleanly extracted and testable — it takes currentPermission, isWindowsTauri, and requestAccess as parameters, making it pure and deterministic.
  • Three Node.js tests cover: Windows Tauri retries denied and accepts granted; non-Windows-Tauri environments keep denied without retrying; default permission still requests on every platform.
  • The E2E test seeds a denied permission with a granted request result on a Win32 platform, toggles desktop notifications off then on, and asserts the request count is 1.
  • The isTauri() import from @tauri-apps/api/core is correct for Tauri 2.
  • No unwrap() or expect() in production paths. No new dependencies.

Rebase result

Head moved from aebe3f29d to d33c71c8c. No conflicts. git range-diff shows the patch is unchanged.

CI

DCO passes. Semgrep OSS and zizmor were pending at the time of this comment.

oceanseth added a commit to oceanseth/buzz that referenced this pull request Aug 7, 2026
fix(desktop): recover Windows notification permission (upstream PR block#2483)
@BradGroux
BradGroux force-pushed the agent/windows-notification-permission-recovery branch from d33c71c to a92cef6 Compare August 7, 2026 21:03
@BradGroux

Copy link
Copy Markdown
Contributor Author

Rebase and accuracy review (2026-08-07)

Rebased onto current main (2b873cf20). Clean fast-forward through the intermediate d42d60d64 base — 52 commits applied, no conflicts.

Branch state: acda0d989 (was d33c71c8c at the intermediate base) Mergeable: ✅ MERGEABLE · BLOCKED (branch protection — awaiting maintainer review)

Accuracy review

The PR adds a ensureDesktopNotificationPermission helper that retries the Windows Tauri notification shim's known false-denied state. When currentPermission is "denied" and the platform is Windows Tauri, it re-requests access instead of accepting the denial. The isWindowsPlatform() helper in platform.ts is tested in platform.test.mjs.

The permission.test.mjs covers both the normal "default" path (request access) and the Windows false-denied path (retry when isWindowsTauri is true). The e2e test in profile.spec.ts exercises the full notification permission flow.

No unwrap()/expect() equivalent in the TypeScript paths. No new dependencies — uses existing Tauri notification plugin APIs. Scope is tight: 7 files, 208 insertions.

⚠️ Note: main has since added PR #5143 (fix(desktop): drop unhandled rejection from throwing window.Notification) which wraps new window.Notification(...) in a try/catch in desktop.ts. This PR's changes to hooks.ts are in a different code path (permission request, not notification delivery) so they do not conflict, but a future merge of both changes should verify the notification flow works end-to-end on Windows after a false denial.

wesbillman
wesbillman previously approved these changes Aug 7, 2026
@BradGroux
BradGroux force-pushed the agent/windows-notification-permission-recovery branch from 6fd06c2 to 4533228 Compare August 8, 2026 12:48
@BradGroux

Copy link
Copy Markdown
Contributor Author

This is the third recurring review + rebase pass for the open BradGroux PRs on block/buzz.

What this PR does

Lets Windows users re-enable desktop notifications after a false denial. On Windows Tauri, the notification permission shim can report "denied" even when the user hasn't actually denied it. The fix introduces an ensureDesktopNotificationPermission function that retries the permission request when the current state is "denied" and the environment is Windows Tauri (but not on other platforms, where "denied" is respected as final).

The PR also adds:

  • isWindowsPlatform() in platform.ts — checks navigator.platform against /^win/i.
  • Test coverage for the permission retry logic (Windows Tauri retries denied, non-Windows-Tauri keeps denied, default always requests).
  • Platform detection tests (Win32 → true, Darwin → false).

Correctness

  • The retry condition currentPermission === "default" || (currentPermission === "denied" && isWindowsTauri) correctly distinguishes the Windows false-denial case from a genuine denial on other platforms.
  • The isWindowsTauri flag combines isWindowsPlatform() && isTauri(), so web browsers on Windows are not affected.
  • The function is pure and testable — requestAccess is injected, making it deterministic in tests.
  • The navigator.platform regex /^win/i matches Win32, Win64, and Windows variants.
  • No new dependencies. No unwrap/expect (TypeScript, no Rust).

Rebase result

Rebased from 2b873cf20 to current main 02f640bc4 (7 commits). Clean rebase, no conflicts.

  • Old head SHA: 6fd06c2a6405e20535cd57cb43f68d31205e522b
  • New head SHA: 4533228ddbadcb4064d818258e86fba93e73a037
  • Mergeable: ✅ MERGEABLE
  • CI: DCO ✅

No code changes were made — this was a review + rebase pass only.

@BradGroux
BradGroux force-pushed the agent/windows-notification-permission-recovery branch from 4533228 to 0720765 Compare August 9, 2026 10:40
@BradGroux

Copy link
Copy Markdown
Contributor Author

Portfolio review update (2026-08-09)

I rechecked Windows notification permission recovery against current main. The retry remains correctly limited to Windows plus Tauri and is not superseded by recent changes.

I rebased the branch onto 5bf78671f45178f8de02ba18d3d321cbbf19cd1f. The reviewed patch remains equivalent and merges cleanly. Existing automated checks were green before the history rewrite and are rerunning. The material remaining validation is still a real packaged Windows 11 notification flow; automated cross-platform checks cannot substitute for that proof.

@MrGrasss

MrGrasss commented Aug 9, 2026

Copy link
Copy Markdown

This still reproduces on Windows 11 with Buzz Desktop 0.5.8, and this PR looks like the fix. Confirmed the false-denial path: OS notifications are on with no per-app or WebView2 denial anywhere (no xyz.block.buzz.app entry under HKCU\...\Notifications\Settings, empty WebView2 permission map, Start Menu shortcut present), yet the app reports "blocked" and the disabled toggle leaves no way to re-request. More detail in #4660. Would be great to get this reviewed and landed. Thanks for the fix.

@BradGroux
BradGroux force-pushed the agent/windows-notification-permission-recovery branch from 0720765 to 6e20e16 Compare August 11, 2026 03:02
@BradGroux

Copy link
Copy Markdown
Contributor Author

Portfolio rebase and review update (2026-08-11)

Rebased onto main at 7e6e9c547 (2026-08-10). No conflicts. The PR branch was also cleaned up — removed a stale review branch that had diverged and force-pushed the canonical agent/windows-notification-permission-recovery branch.

Still needed: Windows notification permission false-denial is still reproducible on Buzz Desktop 0.5.9. The notification-related fixes in #5143 (drop unhandled rejection from window.Notification) and #5004 (skip native notifications outside app bundles) did not address the false denied state before Windows registers the app as a notification sender. The settings toggle still treats that value as final.

Verification: cargo check not applicable (TS only). The PR adds a Windows-specific one-shot recovery request in the Tauri app, a permission.ts module with 60 lines of tests, a platform.ts Windows detection helper with 34 lines of tests, and 44 lines of E2E test coverage in profile.spec.ts.

@Joxyko

Joxyko commented Aug 11, 2026

Copy link
Copy Markdown

Field verification on a real Windows 11 install (packaged 0.5.9)

This is the packaged-shell confirmation this PR and #2445 have been waiting on.

To be precise about what this does and does not prove: I did not build this branch. I verified the mechanism this PR depends on against a stock release build, so the open question -- "on a real Windows 11 / WebView2 install, does the request actually return granted, is the app registered, and does a toast arrive?" -- is answered. The branch's own logic is still covered only by your unit and Playwright tests.

Environment

  • Buzz Desktop 0.5.9, installed release build (%LOCALAPPDATA%\Buzz\buzz-desktop.exe)
  • Windows 11 Home 10.0.26200
  • WebView2 Evergreen runtime 151.0.4129.72
  • Attached with WEBVIEW2_ADDITIONAL_BROWSER_ARGUMENTS=--remote-debugging-port=<free port>, driven over CDP Runtime.evaluate

1. The false denial reproduces on a clean boot

Launched with nothing injected, then waited for the app to mount:

window.Notification.permission                        -> "denied"
window.Notification.permission === "granted"          -> false
localStorage["buzz-notification-settings.v2:<pubkey>"]
  .desktopEnabled                                     -> false

2. A single requestPermission() repairs it

await window.Notification.requestPermission()         -> "granted"
window.Notification.permission                        -> "granted"

One call, no loop -- exactly the one-time recovery this PR performs.

3. A real toast arrives

Fired through the app's own delivery path, new window.Notification(...), which is the same call sendDesktopNotification makes:

ToastNotificationManager::History.GetHistory("xyz.block.buzz.app")
  before -> TOTAL=3
  after  -> TOTAL=4
CreateToastNotifier("xyz.block.buzz.app").Setting     -> Enabled

Windows-side state, for the record. The OS was permissive the entire time:

HKCU\...\CurrentVersion\PushNotifications             ToastEnabled -> no value (not disabled)
HKCU\...\Notifications\Settings\xyz.block.buzz.app    -> present, no `Enabled` value
  LastNotificationAddedTime                           -> 2026-08-07 23:41:38
4 notification policy paths (HKCU+HKLM, Explorer and PushNotifications) -> none exist

So all three acceptance criteria from the PR description hold: the request returns granted, the app is registered as a notification sender, and a real toast is delivered.

A note for #4660, which reports the app is absent from HKCU\...\Notifications\Settings: on this machine the key is present, and the false denial still happens. The two are independent -- the shim never consults the OS either way, so registration state is not what drives the bug.

4. One residual gap this PR does not close

getDesktopNotificationPermissionState() is unchanged, so the boot-time read is still denied, and this effect in hooks.ts fires on every launch before the user touches anything:

React.useEffect(() => {
  if (
    settings.desktopEnabled &&
    (permission === "denied" || permission === "unsupported")
  ) {
    setSettings((current) => ({ ...current, desktopEnabled: false }));
  }
}, [permission, settings.desktopEnabled]);

Measured: with desktopEnabled persisted as true and permission granted in one run, a clean relaunch comes back with desktopEnabled persisted as false. That is where line 3 of section 1 comes from -- alerts had been successfully enabled in the previous run.

This does not detract from the PR: with it, a Windows user can turn alerts on, which is precisely what #2445 asked for and what is broken today. But they would have to do it again after every restart, because the boot-time state is still the false denied and the effect writes the toggle off first.

Closing that too would mean applying the same Windows-Tauri recovery to the mount-time read -- or having getDesktopNotificationPermissionState() report default rather than denied under Windows Tauri, so the existing default branch handles it and ensureDesktopNotificationPermission stays the single decision point.

Happy to open that as a separate issue or a follow-up PR on top of this one. I did not want to widen this PR's scope while it is waiting on review.

5. A correction to the alternative suggested in #2445

#2445 offers a second variant: "getDesktopNotificationPermissionState could prefer the plugin's isPermissionGranted() over window.Notification.permission when running under Tauri."

That variant would not work. The plugin's isPermissionGranted() carries the same short-circuit as the init shim:

// plugins/notification/guest-js/index.ts
async function isPermissionGranted(): Promise<boolean> {
  if (window.Notification.permission !== 'default') {
    return await Promise.resolve(window.Notification.permission === 'granted')
  }
  return await invoke('plugin:notification|is_permission_granted')
}

By the time anything calls it, the init script has already stamped denied, so it returns false without ever reaching the backend invoke -- measured as line 2 of section 1. requestPermission() is the only path that reaches the Rust side, where request_permission() returns PermissionState::Granted unconditionally on desktop (plugins/notification/src/desktop.rs:61-63).

This PR picked the variant that works.

@Joxyko

Joxyko commented Aug 11, 2026

Copy link
Copy Markdown

@BradGroux heads-up so this does not arrive as a surprise: I opened #5600 for the residual gap I described above, rather than pushing into your branch uninvited.

It overlaps yours. It repairs the same false denial one layer earlier, in getDesktopNotificationPermissionState(), which also covers the mount-time read -- and therefore the per-restart regression -- and sendDesktopNotification(), which gates delivery on === "granted". Both PRs add isWindowsPlatform() to platform.ts, so whichever lands second needs a trivial rebase.

I have no attachment to mine landing. If maintainers prefer your shape -- and ensureDesktopNotificationPermission is a nicer seam than my inline condition -- the equivalent is calling your helper from refreshPermission as well as from setDesktopEnabled. Say the word and I will close #5600 in favour of that.

Co-authored-by: Brad Groux <bradgroux@hotmail.com>
Signed-off-by: Brad Groux <bradgroux@hotmail.com>
@BradGroux
BradGroux force-pushed the agent/windows-notification-permission-recovery branch from 6e20e16 to e549b6f Compare August 12, 2026 02:52
@BradGroux

Copy link
Copy Markdown
Contributor Author

@Joxyko — thank you for the exceptional field verification. The CDP-driven testing against a packaged 0.5.9 install on real Windows 11 is exactly the confirmation this PR needed, and your correction about isPermissionGranted() short-circuiting on the init shim's denied is a valuable finding that saved a dead-end approach.

You identified the one residual gap clearly: the boot-time mount-time read in hooks.ts still saw denied and the effect wrote off desktopEnabled=true on every relaunch. I have now closed that gap in this PR.

The fix applies the same ensureDesktopNotificationPermission recovery to refreshPermission in hooks.ts — the same helper the toggle already uses. When running under Windows Tauri, the mount-time read now calls requestPermission() once before the effect can write off the persisted setting, so desktopEnabled=true survives a clean relaunch.

Added tests:

  • permission.test.mjs: boot-time recovery test (Windows Tauri retries a false denied and accepts the granted result) and a guard test (non-Windows-Tauri environments keep denied without re-requesting)
  • platform.test.mjs: Windows platform detection tests
  • profile.spec.ts: E2E test verifying boot-time persisted notifications survive relaunch

I also rebased the entire branch onto the latest main (4b35706, the 0.5.10 release).

This keeps ensureDesktopNotificationPermission as the single decision point, which aligns with your suggestion. The PR now covers both paths: the user-initiated toggle and the boot-time mount-time read.

Thanks again for the detailed Windows-side state analysis. The independence of the HKCU\...\Notifications\Settings registration from the false-denial bug is an important clarification for #4660.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants